Repository navigation
chore: rolling promotion dev -> main - #2729
namastex888 wants to merge 27 commits into
Conversation
Apply risk-proportional validation across target repositories while preserving aggregate integration and release gates.
Cross-machine resume for the roadmap board without committing the binary WAL database: `genie task export --write` snapshots the full genie.db state to git-tracked .genie/roadmap.json, and `genie task import` (--replace to overwrite) losslessly rebuilds genie.db from it on a fresh clone. Import validates snapshot shape and schemaVersion, refuses a non-empty database by default, and preserves row ids exactly. INDEX.md carries the discoverable pointer for agents. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Regenerates the committed board snapshot from genie.db on every commit (fail-open, skipped in linked worktrees and db-less fresh clones) so the published roadmap never lags the live board. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
genie.db is now a materialization of the committed .genie/roadmap.json. A machine-local hash baseline (.genie/roadmap-sync, gitignored) lets `genie task sync` detect which side moved: pulled snapshot -> auto-import, local board mutations -> export, both moved -> refuse and warn with the two explicit resolutions (import --replace / export --write). Every board/task verb auto-syncs fail-open and silently; pre-commit runs sync instead of blind export, so a stale local db can never clobber the published roadmap. Fresh clone bootstraps with any board/task command. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
|
Caution The consumer version of Gemini Code Assist on GitHub has been sunset. All code review activity has officially ceased. |
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
📝 Walkthrough📝 Walkthrough🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches 💡 1📝 Generate docstrings 💡
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: a778482881
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| const dbChanged = marker === null || dbHash !== marker.dbHash; | ||
|
|
||
| if (fileChanged && (!dbChanged || !hasOperationalState(db))) { | ||
| importState(db, parsed, { replace: true }); |
There was a problem hiding this comment.
When a pull-triggered import overlaps another process updating the shared database, that write can commit after dbHash is calculated on line 103 but before this replacement transaction begins; importState(..., { replace: true }) then deletes the newly committed task state even though the sync classified the database as unchanged. Recheck the database hash while holding the write transaction, or otherwise serialize the comparison and replacement, so parallel workers cannot lose updates during auto-sync.
AGENTS.md reference: AGENTS.md:L44-L44
Useful? React with 👍 / 👎.
| git_common=$(git rev-parse --path-format=absolute --git-common-dir 2>/dev/null) | ||
| if [ "$git_dir" = "$git_common" ] && [ -f .genie/genie.db ] && [ -f src/genie.ts ]; then | ||
| if bun src/genie.ts task sync; then | ||
| [ -f .genie/roadmap.json ] && git add .genie/roadmap.json |
There was a problem hiding this comment.
When .genie/roadmap.json already has valid unstaged edits, task sync imports them successfully and this unconditional git add puts the entire file into an otherwise unrelated commit. This breaks partial-staging workflows and violates the repository requirement to preserve unrelated dirty-worktree changes; stage the file only when the hook itself exported a refreshed snapshot, without absorbing pre-existing edits.
AGENTS.md reference: AGENTS.md:L31-L31
Useful? React with 👍 / 👎.
| export function importState(db: Database, snapshot: unknown, opts: { replace?: boolean } = {}): ImportSummary { | ||
| const state = validateSnapshot(db, snapshot); | ||
| const apply = db.transaction(() => { | ||
| if (hasOperationalState(db)) { |
There was a problem hiding this comment.
When the destination has no operational rows but still contains metadata such as wish_sig:*, hasOperationalState(db) is false, so even import --replace skips the wipe and merges snapshot metadata with stale local keys. The resulting export is not a lossless copy of the snapshot, and a retained wish signature can later cause a false drift error; replacement imports should clear all tables regardless of whether operational rows currently exist.
Useful? React with 👍 / 👎.
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 3
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@src/lib/v5/roadmap-sync.ts`:
- Around line 99-148: Add cross-process locking around the entire
read-decide-write sequence in syncRoadmap, including roadmap.json parsing,
marker reads, database import/export decisions, and file/marker writes. Use an
advisory lock associated with the sync marker path, and ensure the lock is
acquired before reading either state and released on every return path,
including invalid JSON and unchanged-state branches; apply the same protection
to autoSyncBeforeCommand.
In `@src/lib/v5/task-state.ts`:
- Around line 1447-1468: Update validateSnapshot and the snapshot insertion flow
around insertSnapshotRows to validate each table row’s expected fields and value
types before binding, and convert any unexpected insertion/bind failure into
SnapshotFormatError with the table and row context. Preserve the transaction
rollback behavior and existing schema validation, while ensuring malformed
roadmap snapshot rows never escape as raw SQLite errors.
In `@src/term-commands/v5-task.ts`:
- Around line 431-436: Normalize the paths before the baseline checks in both
handleExport and handleImport. Compare the resolved canonical roadmap path with
the resolved target/source path, while preserving the existing
recordSyncBaseline(db) behavior when they refer to the same file regardless of
relative or absolute spelling.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro Plus
Run ID: 8186e0ff-d395-4ce6-9aa5-2ec415aa11dc
⛔ Files ignored due to path filters (1)
CLAUDE.mdis excluded by!*.md
📒 Files selected for processing (16)
.claude-plugin/marketplace.json.genie/INDEX.md.genie/roadmap.json.gitignore.husky/pre-commitpackage.jsonplugins/genie/.claude-plugin/plugin.jsonplugins/genie/.codex-plugin/plugin.jsonplugins/genie/package.jsonplugins/hermes-genie/plugin.yamlsrc/lib/v5/genie-db.tssrc/lib/v5/roadmap-sync.tssrc/lib/v5/task-state.tssrc/term-commands/v5-board.tssrc/term-commands/v5-task.test.tssrc/term-commands/v5-task.ts
| export function syncRoadmap(db: Database, cwd?: string): SyncResult { | ||
| const filePath = resolveRoadmapPath(cwd); | ||
| const markerPath = resolveSyncMarkerPath(cwd); | ||
| const dbState = exportState(db); | ||
| const dbHash = canonicalHash(dbState); | ||
|
|
||
| if (!existsSync(filePath)) { | ||
| if (!hasOperationalState(db)) return { action: 'none' }; | ||
| writeFileSync(filePath, serializeSnapshot(dbState)); | ||
| writeMarker(markerPath, { fileHash: dbHash, dbHash }); | ||
| return { action: 'exported', message: `Published board snapshot to ${filePath}.` }; | ||
| } | ||
|
|
||
| let parsed: unknown; | ||
| try { | ||
| parsed = JSON.parse(readFileSync(filePath, 'utf-8')); | ||
| } catch { | ||
| return { | ||
| action: 'diverged', | ||
| message: `${filePath} is not valid JSON. Fix it, or regenerate it from the local board with \`genie task export --write\`.`, | ||
| }; | ||
| } | ||
| const fileHash = canonicalHash(parsed); | ||
|
|
||
| if (fileHash === dbHash) { | ||
| writeMarker(markerPath, { fileHash, dbHash }); | ||
| return { action: 'none' }; | ||
| } | ||
|
|
||
| const marker = readMarker(markerPath); | ||
| const fileChanged = marker === null || fileHash !== marker.fileHash; | ||
| const dbChanged = marker === null || dbHash !== marker.dbHash; | ||
|
|
||
| if (fileChanged && (!dbChanged || !hasOperationalState(db))) { | ||
| importState(db, parsed, { replace: true }); | ||
| writeMarker(markerPath, { fileHash, dbHash: canonicalHash(exportState(db)) }); | ||
| return { action: 'imported', message: `Board refreshed from ${filePath}.` }; | ||
| } | ||
| if (dbChanged && !fileChanged) { | ||
| writeFileSync(filePath, serializeSnapshot(dbState)); | ||
| writeMarker(markerPath, { fileHash: dbHash, dbHash }); | ||
| return { action: 'exported', message: `Board snapshot ${filePath} refreshed from the local database.` }; | ||
| } | ||
| const resolution = | ||
| 'Resolve with `genie task import --replace` (take the snapshot) or `genie task export --write` (keep the local board).'; | ||
| return { | ||
| action: 'diverged', | ||
| message: `Both the local board (genie.db) and ${filePath} changed since the last sync. Nothing was overwritten. ${resolution}`, | ||
| }; | ||
| } |
There was a problem hiding this comment.
🩺 Stability & Availability | 🟠 Major | 🏗️ Heavy lift
No cross-process locking around the roadmap.json/marker read-decide-write sequence.
syncRoadmap reads the marker and roadmap.json, decides an action, then writes the file and/or marker (Lines 105-148) — and autoSyncBeforeCommand (Lines 156-170) runs this before "every board/task command" per its docstring, plus the pre-commit hook per the PR objectives. Only the DB-side mutation inside importState is protected by a SQLite transaction; the file reads/writes here have no lock. Two processes racing (e.g. a pre-commit hook firing while an interactive genie task command is mid-sync) can interleave reads and writes on roadmap.json/the marker file, defeating the "diverged branch is the whole point" guarantee the module doc describes.
Worth a file lock (e.g. flock-style advisory lock on the marker path) around the read-decide-write critical section, or an explicit documented single-process assumption if this is accepted as out of scope for now.
Also applies to: 156-170
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@src/lib/v5/roadmap-sync.ts` around lines 99 - 148, Add cross-process locking
around the entire read-decide-write sequence in syncRoadmap, including
roadmap.json parsing, marker reads, database import/export decisions, and
file/marker writes. Use an advisory lock associated with the sync marker path,
and ensure the lock is acquired before reading either state and released on
every return path, including invalid JSON and unchanged-state branches; apply
the same protection to autoSyncBeforeCommand.
…lways valid evidence - work/review: a passing full-suite run counts as evidence; a missing scope rationale is a write-up gap (at most MEDIUM), never by itself grounds for FIX-FIRST - wish/work/review: a repository-documented gate is by itself sufficient justification for its scope - align dispatch-brief validation example and wish acceptance criterion - mirrors regenerated via sync-plugin-skills --write
feat(skills): make validation proportional across repositories
…p content Drop the preAction auto-sync from task/board commands — reconciliation now runs where state actually moves: post-merge/post-rewrite import a pulled snapshot, pre-commit exports local board changes, both fail-open and divergence-refusing. Scope the published snapshot to roadmap content: hire_roster (machine-local worktree paths) is excluded from roadmap.json and preserved untouched by canonical imports; plain `task export` stdout remains the complete database dump. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
There was a problem hiding this comment.
Actionable comments posted: 1
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
src/lib/v5/roadmap-sync.ts (1)
147-151: 🗄️ Data Integrity & Integration | 🟠 Major | ⚡ Quick winDb-changed export path can silently blow away a populated
roadmap.json.Unlike the "file missing" branch above (Line 116), which checks
hasOperationalState(db)before writing, this branch exports wheneverdbChanged && !fileChangedwith no such guard. If the localgenie.dbis ever wiped/recreated (corruption, accidental delete) while the marker still reflects a previously-populated state, this treats it as "db changed" and overwrites the canonical, git-trackedroadmap.jsonwith an effectively empty snapshot — which the pre-commit hook could then commit, destroying the shared roadmap for the whole team. This directly contradicts the module's stated "no silent overwrites" design.🛡️ Proposed fix
- if (dbChanged && !fileChanged) { + if (dbChanged && !fileChanged && hasOperationalState(db)) { writeFileSync(filePath, serializeSnapshot(dbState)); writeMarker(markerPath, { fileHash: dbHash, dbHash }); return { action: 'exported', message: `Board snapshot ${filePath} refreshed from the local database.` }; }🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/lib/v5/roadmap-sync.ts` around lines 147 - 151, Guard the dbChanged && !fileChanged export branch with hasOperationalState(db), matching the file-missing path before writing roadmap.json or its marker. When the database lacks operational state, skip the export so an empty or recreated genie.db cannot overwrite a populated snapshot; preserve the existing export behavior for operational databases.src/term-commands/v5-task.test.ts (1)
446-491: 🎯 Functional Correctness | 🟡 Minor | ⚡ Quick winDivergence test no longer asserts the stderr warning format.
The stderr
warn:assertion was dropped from the divergence scenario, leaving only the local-state check. This is the exact refusal path pre-commit relies on to decide whether to warn the user — losing stderr coverage here weakens regression protection for that message format.As per path instructions, "Every new CLI surface must test success, error exit code, stderr, and idempotency."
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/term-commands/v5-task.test.ts` around lines 446 - 491, The divergence scenario in the test “pulled snapshot imports on sync; local mutation exports; divergence is refused then resolvable” must also validate the refusal warning on stderr. After invoking sync for the diverged state, assert the expected `warn:` message format alongside the existing exit-code, stdout, snapshot, and local-state assertions.Source: Coding guidelines
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@src/lib/v5/task-state.ts`:
- Around line 1580-1586: Update the snapshot wipe flow around insertSnapshotRows
so preserveHireRoster truly protects existing hire_roster rows when wish_groups
and boards are deleted and recreated. Inspect the hire_roster schema and
foreign-key relationships, then prevent those deletions from cascading into or
violating preserved hires while retaining the current wipe behavior for
non-preserved data.
---
Outside diff comments:
In `@src/lib/v5/roadmap-sync.ts`:
- Around line 147-151: Guard the dbChanged && !fileChanged export branch with
hasOperationalState(db), matching the file-missing path before writing
roadmap.json or its marker. When the database lacks operational state, skip the
export so an empty or recreated genie.db cannot overwrite a populated snapshot;
preserve the existing export behavior for operational databases.
In `@src/term-commands/v5-task.test.ts`:
- Around line 446-491: The divergence scenario in the test “pulled snapshot
imports on sync; local mutation exports; divergence is refused then resolvable”
must also validate the refusal warning on stderr. After invoking sync for the
diverged state, assert the expected `warn:` message format alongside the
existing exit-code, stdout, snapshot, and local-state assertions.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro Plus
Run ID: 445ad893-b474-44ce-bfe7-b263c8e07ca9
⛔ Files ignored due to path filters (1)
CLAUDE.mdis excluded by!*.md
📒 Files selected for processing (8)
.genie/INDEX.md.husky/post-merge.husky/post-rewrite.husky/pre-commitsrc/lib/v5/roadmap-sync.tssrc/lib/v5/task-state.tssrc/term-commands/v5-task.test.tssrc/term-commands/v5-task.ts
…s exempt Maintainer revision of the bypass policy, rebased onto dev: - every bypass is announced in one line with a 'use genie' override - security-sensitive changes keep lifecycle review gates by default; only an explicit user choice skips them - bug reports (report), operational commands, and Genie questions are cheap and never bypassed — resolves the contradiction with the intent-classification table and skill catalog - value-gate criteria get a worked example each; uncertainty inspection bounded to the two ls commands plus files the request names - skill description keeps bug/question routing discoverable - content-contract test pins policy invariants instead of full sentences - mirrors regenerated via sync-plugin-skills --write
fix(router): bypass Genie when lifecycle adds no value
…onical board Design SHIP (1 fix loop, digest 36097b0d…), plan SHIP (2 fix loops, all findings execution-verified). 3-scout audit: 29 wishes DONE-verified on dev, 5 in progress, 2 open. Plan: archive 29 with link-rewrite inventory, cold-seed roadmap board with 19 lifecycle cards (Felipe-ratified work order), rewrite INDEX with Shipped section, draft public-roadmap-polish. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
|
@codex review this pr |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: e7a1dfb68d
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| if [ "$git_dir" = "$git_common" ] && [ -f src/genie.ts ] && [ -f .genie/roadmap.json ]; then | ||
| bun src/genie.ts task sync || true |
There was a problem hiding this comment.
When the main checkout switches between branches containing different .genie/roadmap.json versions, neither this hook nor post-rewrite runs, so the shared genie.db continues exposing the previous branch's board until a manual sync or pre-commit. Any intervening task list, claim, or mutation therefore operates on stale state and can cause divergence; the checked .husky hook set needs the same reconciliation in a user post-checkout hook, which Git documents as running after checkout updates the worktree.
Useful? React with 👍 / 👎.
| // The canonical snapshot is roadmap-scoped: local hires stay untouched. | ||
| const canonical = source === resolveRoadmapPath(); | ||
| const summary = importState(db, snapshot, { replace: opts.replace, preserveHireRoster: canonical }); |
There was a problem hiding this comment.
When a user runs genie task import .genie/roadmap.json --replace, source is relative while resolveRoadmapPath() is absolute, so this comparison marks the actual canonical snapshot as non-canonical. The import consequently wipes the machine-local hire_roster and skips recording the sync baseline, even though omitting the identical filename preserves hires; compare normalized absolute paths rather than raw strings.
Useful? React with 👍 / 👎.
| // --write publishes the roadmap slice (no hire_roster — machine-local | ||
| // worktree paths); the plain stdout dump stays the complete database. | ||
| const state = opts.write ? roadmapSnapshot(db) : exportState(db); |
There was a problem hiding this comment.
When genie task export --write backup.json is used, this branch selects roadmapSnapshot() and silently removes hire_roster, whereas plain task export emits complete state and importing a custom file treats it as a full snapshot. An ensuing task import backup.json --replace therefore deletes all hire/worktree bindings, so only the canonical roadmap export should omit hires or the custom-file import/export contract must preserve them consistently.
Useful? React with 👍 / 👎.
| @@ -0,0 +1,16 @@ | |||
| model: | |||
| default: gpt-5.6-sol | |||
There was a problem hiding this comment.
Because this repository-local Hermes profile uses the openai-codex provider, setting a release-specific model ID forces every session away from the client's active model and will fail or become stale where that identifier is unavailable. Let the provider inherit the active model instead of committing this pin.
AGENTS.md reference: AGENTS.md:L37-L37
Useful? React with 👍 / 👎.
…lugin is enabled syncClaude computed pluginEnabled and used it to suppress the bare-name skills mirror, but never passed it to syncClaudeAgentFiles. Role agents were fanned into ~/.claude/agents unconditionally while the plugin also served the same files as genie:*, so all seven double-listed in the agent picker — the exact waste the skills-mirror hotfix removed. Pass pluginEnabled through and feed syncClaudeAgentFiles an empty source set when it is true. Every manifest-owned file then flows through the existing planAgentOrphan path, which backs the bytes up before retiring them and preserves hand-edited files as kept-modified-orphan. The GENIE_FORCE_CLAUDE_SKILLS_MIRROR kill-switch already gates both surfaces because it lives inside claudeGeniePluginEnabled. Doctor is updated to match: with the plugin enabled the expected source set is empty, so a surviving fan-out reads as prunable stale rather than current, and a pruned one passes instead of reporting seven missing-from-target and advising `genie update` forever. The duplicate surface warning now tracks leftover files rather than merely the plugin being enabled, and points at the sync suggestion that actually resolves it. checkRoleAgents now uses claudeGeniePluginEnabled, dropping a duplicate settings reader that ignored the kill-switch.
…ware fix(agent-sync): suppress the bare-name role-agent fan-out when the plugin is enabled
There was a problem hiding this comment.
Actionable comments posted: 8
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
src/genie-commands/doctor.ts (1)
1108-1117: 🎯 Functional Correctness | 🟠 Major | ⚡ Quick winDetect unmanaged role-agent files while suppression is active.
When
suppressedemptiessource,namesis built only from source keys and manifest entries. A fresh user-createdagents/reviewer.mdwith no manifest entry is therefore never classified, soduplicateSurfacestays false even thoughgenie:reviewerand barereviewerare both visible. Retain the source role-name set for discovery while using an empty digest map for suppression comparisons, or otherwise include matching on-disk files.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/genie-commands/doctor.ts` around lines 1108 - 1117, Update the agent discovery logic around sourceState, source, and names so suppression uses an empty digest map for comparison but retains the unsuppressed source role-name set for discovery. Include matching on-disk bare role-agent files such as reviewer.md even without manifest entries, allowing duplicateSurface to detect both genie:reviewer and reviewer while preserving prunable-leftover handling.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In @.genie/brainstorms/genie-official-roadmap/DESIGN.md:
- Around line 88-95: Refresh the Reviewed content SHA-256 in the bounded
genie-design-review block to match the current DESIGN.md content with that block
removed. Preserve the existing SHIP verdict and review metadata, and only retain
the verdict once the digest is accurate.
In @.genie/wishes/genie-official-roadmap/WISH.md:
- Around line 129-130: Strengthen the roadmap validation commands in the
documented checks: replace aggregate task-count and non-null-lane assertions
with exact expected title/lane pairs for all roadmap tasks, and update the
doctor validation to filter results to the expected roadmap slugs before
asserting the required number of ok entries. Apply the same correction to the
additional check at the referenced later section.
- Line 191: Propagate the post-approval execution amendment throughout the
earlier plan for genie-official-roadmap: update every enumeration, command,
count, and acceptance criterion that places its own card in Brainstorm or its
INDEX entry in Raw, including the sections around the referenced plan lines, to
use Wish and Poured respectively. Preserve all other placements and ensure the
final board/INDEX checks and ok>=18 threshold reflect the amended state.
In @.hermes/config.yaml:
- Around line 1-4: Update the model.default setting in the Hermes configuration
to use the documented active-model inheritance value instead of hardcoding
gpt-5.6-sol. Keep the openai-codex provider and base_url unchanged, and ensure
Codex agents inherit the caller’s active session model.
In @.hermes/SOUL.md:
- Around line 18-20: Update the command guidance in SOUL.md to prohibit only the
obsolete top-level genie serve command, while explicitly allowing the supported
genie omni serve command documented in the lifecycle references. Keep the
existing prohibitions for the other obsolete surfaces unchanged.
In `@scripts/release-docs.test.ts`:
- Around line 976-993: Update the test “router pays Genie lifecycle cost only
when it adds value” to assert that the router metadata loaded from
“skills/genie/agents/openai.yaml” contains the security-sensitive bypass rule,
matching the existing SKILL.md and lifecycle assertions.
In `@skills/genie/SKILL.md`:
- Around line 49-53: Update the State Detection guidance in both
skills/genie/SKILL.md (lines 49-53) and plugins/genie/skills/genie/SKILL.md
(lines 49-53) to read the matched WISH.md or brainstorm handoff, within a
bounded scope, before applying the status-specific routing override; retain the
existing slug listing/matching and routing behavior in both copies.
In `@src/lib/agent-sync.ts`:
- Around line 5674-5682: Update syncClaudeAgentFiles to return a status
indicating whether synchronization committed changes or is already converged,
including unsafe-manifest and committed === false outcomes. In the caller, emit
the agents-mirror suppressed report entry only when that returned status
confirms a committed or already-converged state; otherwise leave the suppression
report absent.
---
Outside diff comments:
In `@src/genie-commands/doctor.ts`:
- Around line 1108-1117: Update the agent discovery logic around sourceState,
source, and names so suppression uses an empty digest map for comparison but
retains the unsuppressed source role-name set for discovery. Include matching
on-disk bare role-agent files such as reviewer.md even without manifest entries,
allowing duplicateSurface to detect both genie:reviewer and reviewer while
preserving prunable-leftover handling.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro Plus
Run ID: c4cb9fe8-a3d5-47df-b70e-79177292c738
📒 Files selected for processing (27)
.claude-plugin/marketplace.json.genie/INDEX.md.genie/brainstorms/genie-official-roadmap/DESIGN.md.genie/brainstorms/genie-official-roadmap/DRAFT.md.genie/roadmap.json.genie/wishes/genie-official-roadmap/WISH.md.gitignore.hermes/SOUL.md.hermes/config.yaml.hermes/profile.yamlbiome.jsonpackage.jsonplugins/genie/.claude-plugin/plugin.jsonplugins/genie/.codex-plugin/plugin.jsonplugins/genie/package.jsonplugins/genie/skills/genie/SKILL.mdplugins/genie/skills/genie/agents/openai.yamlplugins/genie/skills/genie/reference/lifecycle.mdplugins/hermes-genie/plugin.yamlscripts/release-docs.test.tsskills/genie/SKILL.mdskills/genie/agents/openai.yamlskills/genie/reference/lifecycle.mdsrc/genie-commands/doctor.test.tssrc/genie-commands/doctor.tssrc/lib/agent-sync.test.tssrc/lib/agent-sync.ts
| genie board --board roadmap && genie task export | bun -e 'const s=JSON.parse(await Bun.stdin.text());const b=s.boards.find(x=>x.name==="roadmap");if(!b)process.exit(1);const c=s.tasks.filter(t=>t.board_id===b.id);if(c.length!==19||c.some(t=>!t.lane))process.exit(1)' | ||
| ``` |
There was a problem hiding this comment.
🗄️ Data Integrity & Integration | 🟠 Major | ⚡ Quick win
Validate the expected roadmap set, not just aggregate counts.
The board check accepts any 19 tasks with non-null lanes, even if titles or lanes are wrong. The doctor check counts 18 ok entries globally, so unrelated entries can satisfy the threshold while the roadmap entries remain unresolved. Assert the exact title/lane pairs and filter doctor results to the expected roadmap slugs.
Also applies to: 154-156
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In @.genie/wishes/genie-official-roadmap/WISH.md around lines 129 - 130,
Strengthen the roadmap validation commands in the documented checks: replace
aggregate task-count and non-null-lane assertions with exact expected title/lane
pairs for all roadmap tasks, and update the doctor validation to filter results
to the expected roadmap slugs before asserting the required number of ok
entries. Apply the same correction to the additional check at the referenced
later section.
| model: | ||
| default: gpt-5.6-sol | ||
| provider: openai-codex | ||
| base_url: https://chatgpt.com/backend-api/codex |
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick win
Do not pin an unstable Codex model identifier.
model.default hardcodes gpt-5.6-sol, which can override the active session model and become unavailable independently of the caller. Use Hermes’ documented active-model inheritance setting instead.
As per coding guidelines, Codex agents inherit the active model and must not hardcode unstable model identifiers.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In @.hermes/config.yaml around lines 1 - 4, Update the model.default setting in
the Hermes configuration to use the documented active-model inheritance value
instead of hardcoding gpt-5.6-sol. Keep the openai-codex provider and base_url
unchanged, and ensure Codex agents inherit the caller’s active session model.
Source: Coding guidelines
| syncClaudeAgentFiles(ctx, claudeDir, report, pluginEnabled); | ||
| if (pluginEnabled) { | ||
| report.extras.push({ | ||
| kind: 'agents-mirror', | ||
| action: 'suppressed', | ||
| detail: | ||
| 'genie@automagik plugin enabled — bare-name role-agent fan-out pruned; agents load as genie:* from the plugin', | ||
| }); | ||
| } |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
Only report suppression after the sync succeeds.
syncClaudeAgentFiles can leave files untouched for an unsafe manifest or record result.committed === false, but this caller still emits agents-mirror: suppressed. The report can therefore claim the duplicate surface was pruned when it remains present. Return a success/status result from the sync function and emit this extra only after a committed or already-converged state.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@src/lib/agent-sync.ts` around lines 5674 - 5682, Update syncClaudeAgentFiles
to return a status indicating whether synchronization committed changes or is
already converged, including unsafe-manifest and committed === false outcomes.
In the caller, emit the agents-mirror suppressed report entry only when that
returned status confirms a committed or already-converged state; otherwise leave
the suppression report absent.
…2732) The test called runManualUpdateConvergence without a selection, so the default resolved the host machine's persisted integration consent (~/.genie/.integration-consent.json). On a codex-consent machine the agent-sync arm narrows to null, runSync never fires, and the test fails — while CI (no consent file) and all/auto machines pass. Same commit, same binary, different machine state. Pass selection: 'all' explicitly, matching every other caller in this file (the codex-only, auto, and hermes-leg tests already pin theirs). Closes #2732
- task export/import: resolve explicit paths before the canonical comparison, so a relative spelling of .genie/roadmap.json preserves local hires and records the sync baseline - task export --write <file>: custom-file backups emit the complete state including hire_roster; only canonical roadmap.json gets the roadmap slice, keeping backup round-trips lossless - importState: --replace wipes all tables even when only meta rows exist, so stale wish_sig/backfill markers cannot merge into the snapshot; malformed rows that pass shape validation rethrow as SnapshotFormatError after rollback - syncRoadmap: compare-and-act runs under BEGIN IMMEDIATE, closing the window where a concurrent task write could be destroyed by a replace-import and serializing concurrent syncs - pre-commit: stage roadmap.json only when sync itself rewrote it, never absorbing pre-existing unstaged edits - post-checkout (new hook): run task sync after branch checkouts so the shared db stops exposing the previous branch's board - .hermes/SOUL.md: narrow the obsolete-serve prohibition so it cannot be read as banning the supported 'genie omni serve' - release-docs.test.ts: assert the security-sensitive bypass rule in router metadata too Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
…solation fix(test): pin selection in the manual post-update convergence test
fix(v5): harden roadmap sync per PR #2729 bot-review findings
- task export/import: resolve explicit paths before the canonical comparison, so a relative spelling of .genie/roadmap.json preserves local hires and records the sync baseline - task export --write <file>: custom-file backups emit the complete state including hire_roster; only canonical roadmap.json gets the roadmap slice, keeping backup round-trips lossless - importState: --replace wipes all tables even when only meta rows exist, so stale wish_sig/backfill markers cannot merge into the snapshot; malformed rows that pass shape validation rethrow as SnapshotFormatError after rollback - syncRoadmap: compare-and-act runs under BEGIN IMMEDIATE, closing the window where a concurrent task write could be destroyed by a replace-import and serializing concurrent syncs - pre-commit: stage roadmap.json only when sync itself rewrote it, never absorbing pre-existing unstaged edits - post-checkout (new hook): run task sync after branch checkouts so the shared db stops exposing the previous branch's board - .hermes/SOUL.md: narrow the obsolete-serve prohibition so it cannot be read as banning the supported 'genie omni serve' - release-docs.test.ts: assert the security-sensitive bypass rule in router metadata too Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Rolling Promotion PR
Auto-maintained rolling promotion PR from
devtomain.Process:
ready-to-mergeadded when all checks passSummary by CodeRabbit
task export,task import, andtask syncfor roadmap-aware snapshot workflows.5.260729.1; updated local sync automation and git ignore rules.